Skip to content

Remove swsssdk from rules and image.#11469

Merged
liuh-80 merged 3 commits intosonic-net:masterfrom
liuh-80:dev/liuh/update-rules
Aug 25, 2022
Merged

Remove swsssdk from rules and image.#11469
liuh-80 merged 3 commits intosonic-net:masterfrom
liuh-80:dev/liuh/update-rules

Conversation

@liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Jul 18, 2022

Why I did it

To deprecate swsssdk, remove all dependency to it.

How I did it

Remove swsssdk from rules and build image scripts.

How to verify it

Pass all UT and E2E test case

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205

Description for the changelog

Remove swsssdk from rules and build image scripts.

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@liuh-80
Copy link
Contributor Author

liuh-80 commented Jul 18, 2022

Remove swsssdk from rules first, if UT passed, will remove from image later.

@liuh-80
Copy link
Contributor Author

liuh-80 commented Aug 1, 2022

/azp run Azure.sonic-buildimage

@azure-pipelines
Copy link

Pull request contains merge conflicts.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 3, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: liuh-80 / name: Hua Liu (81e977038183c68eb0b231f0ad0eab7cbf5f16ce)

@liuh-80 liuh-80 force-pushed the dev/liuh/update-rules branch from 3c2596e to 81e9770 Compare August 10, 2022 03:49
Copy link
Contributor Author

@liuh-80 liuh-80 Aug 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Database container depends on redis and redis-dump-load
And after remove SWSSSDK_PY2, the indirect depency also been removed, so some test case failed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this removal also deprecate the redis-dump-load functionality?
What if users still want to use redis-dump-load?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our container does not have redis-dump. We have include this change: #12185

Is there other changes that we are missing?

Copy link
Contributor

@mint570 mint570 Mar 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it is missing the entry script:

ls /usr/local/bin/redis-dump

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we are using sonic-db-dump instead of redis-dump now.
Are they basically the same thing?
Are we removing redis-dump?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand now. sonic-db-dump-load is built on top of redis-dump-load. So it needs redis-dump-load as dependency. Currently, for sonic_py_common python3 package, it does not list redis-dump-load as dependency. And hence sonic-db-dump-load doesn't work. This needs to be fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mint570 , here is the fix PR: #14347

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mint570, may I know how you found this issue, some command failed or errro in syslog? I will create a sonic-mgmt test case to prevent this happen again, so need understand the break scenario.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just called sonic-db-dump in the container, it failed with the redisdl import.
(Initially I was looking for redis-dump, but there is no such tool in the container.)

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 11, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: liuh-80 / name: Hua Liu (1e621d2)

@liuh-80 liuh-80 force-pushed the dev/liuh/update-rules branch from 5e09524 to 1e621d2 Compare August 11, 2022 06:32
@liuh-80 liuh-80 changed the title [WIP] Remove swsssdk from rules and image. Remove swsssdk from rules and image. Aug 15, 2022
@liuh-80 liuh-80 marked this pull request as ready for review August 15, 2022 03:13
@liuh-80 liuh-80 requested review from lguohan and xumia as code owners August 15, 2022 03:13
@liuh-80
Copy link
Contributor Author

liuh-80 commented Aug 15, 2022

This PR need wait for 3 vendor side change complete first.
Because this change will remove swsssdk from dockers, and if any vendor's code running in docker still suing swsssdk, their code will break.

@liuh-80
Copy link
Contributor Author

liuh-80 commented Aug 15, 2022

Will check if can move some change which does not depend on vender code to another PR.

@liuh-80
Copy link
Contributor Author

liuh-80 commented Aug 24, 2022

WIll fix merge conflict and update soon.

@liuh-80 liuh-80 merged commit 214e394 into sonic-net:master Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants